modules/zstd: Add support for decoding compressed blocks by lpawelcz · Pull Request #1857 · google/xls

lpawelcz · 2025-01-15T16:32:34Z

This PR extends the ZstdDecoder with support for decoding compressed blocks.

It supersedes PRs:

The decoder is capable of decoding RAW and RLE literals as well as sequences with predefined FSE tables.
A suite of DSLX tests comprising unit tests of all underlying procs and an integration test was prepared.
The integration test, similarly as in #1654, first generates a random valid ZSTD frame with compressed blocks and expected decoded output. Test data is then converted to a DSLX file (example) that is imported by the integration tests file.
At the beginning of the test, the default FSE decoding tables are filled with default distributions taken from RFC 8878 section 3.1.1.3.2.2. Default Distributions . Next, the encoded frame is loaded to the system memory and the decoder is configured through a set of CSRs to start the decoding process. The decoder starts the operation and writes the decoded frame back into the output buffer in the system memory. Once it finishes, it sends a pulse on the notify channel signaling the end of the decoding. The output of the decoder is compared against the decoding result from the reference library.

The PR introduces among others:

CompressedBlockDecoder - manages both SequenceDecoder and LiteralsDecoder to enable compress block decoding. Integrated with the top-level ZstdDecoder
SequenceDecoder - responsible for decoding sequence sections of the compressed blocks
FseDecoder - introduced as the core part of the SequenceDecoder
RefillingShiftBuffer - used for storing and outputting in forward and backward fashion an arbitrary amount of bits required by the FSE decoder
LiteralsDecoder - capable of decoding RAW, RLE and Huffman-coded literals
HuffmanDecoder - used in decoding huffman-coded literals. Decoded Huffman trees are then used to decode one or four Huffman-coded streams.
CommandConstructor - this proc is responsible for sending packets with decoded sequences and literals to the SequenceExecutor proc
RamMux and RamDemux - procs used for handling requests/responses to multiple memory models. The procs interface with 3 separate memory buffers for FSE decoding tables.

google-cla · 2025-01-15T16:32:43Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

proppy · 2025-01-27T01:25:00Z

Can we rebase and consolidate with #1654 ?

lpawelcz · 2025-02-05T09:15:24Z

Can we rebase and consolidate with #1654 ?

@proppy done
This PR also supersedes #1616 so I also closed it in favor of this one

xls/dslx/stdlib/BUILD

proppy · 2025-03-06T08:16:44Z

xls/modules/zstd/zstd_frame_dslx.py

@@ -0,0 +1,170 @@
+# Copyright 2024 The XLS Authors


can. you add a docstring about what the module is doing and where it is being used?

Added the docstring

proppy · 2025-03-06T08:17:37Z

xls/modules/zstd/external/BUILD

@@ -0,0 +1,33 @@
+# Copyright 2024 The XLS Authors


can. you add a docstring about what those external are and why they are needed?

Added the docstring and a License file for the third party sources

proppy · 2025-03-06T08:18:08Z

xls/modules/rle/rle_common.x

    last: bool,               // flush RLE
 }

+// Structure contains multiple uncompressed symbols.


is that used by zstd?

It seems like it is not used in this version of the ZSTD Decoder. Let's just remove it for now.

proppy · 2025-03-06T08:18:56Z

xls/modules/zstd/zstd_dec_wrapper.v

@@ -0,0 +1,753 @@
+// Copyright 2024 The XLS Authors


can. you add a docstring about what the module is and why it is needed? (if it's for driving the cocostb test maybe move it there or in a separate rtl folder?)

Moved to a separate rtl directory and added short description

proppy · 2025-03-06T08:19:37Z

xls/modules/zstd/memory/mem_writer_wrapper.v

@@ -0,0 +1,193 @@
+// Copyright 2024 The XLS Authors


can you move the verilog file in a separate rtl directory w/ a README (or docstring describing the modules)?

Moved to a separate rtl directory and added short description

proppy · 2025-03-06T08:20:11Z

can you also rebase?

proppy · 2025-04-09T14:43:57Z

dependency_support/pip_requirements.in

+cocotb==1.9.0
+cocotbext-axi==0.1.24
+cocotb_bus==0.2.1
+zstandard==0.23.0


q: does this need to be in sync w/ the C++ dep?

Since we dropped the C++ tests for the ZSTD Decoder I believe those don't have to be in sync

lpawelcz · 2025-04-09T14:49:25Z

Addressed the review comments, apart from that:

It was required to remove materialize_internal_fifos for procs with loopback channels
- Explicitly set materialize_internal_fifos=false for offending procs and provided verilog implementation of the fifo for the synthesis targets
- Added the option to procs that had it missing
Renamed axi_ram.x file to axi_ram_reader.x
Removed CC tests step from the ZSTD module CI workflow
- No longer applicable as there are no C++ tests in ZSTD now
Added missing codegen options (mostly module_name)
Removed Reset CSR
- Adjusted DSLX code, tests and the docs

proppy · 2025-04-09T15:09:41Z

xls/modules/zstd/data/comp_frame_huffman.x

missing license?

Done - there's a license header now.

proppy · 2025-04-09T15:10:56Z

xls/modules/zstd/external/axi_crossbar_addr.v

@@ -0,0 +1,418 @@
+/*


I think this has to go in third_party, since it is not authored by a contributor.

This has been moved to third_party/verilog_axi.

proppy · 2025-04-09T15:11:17Z

xls/modules/zstd/external/axi_crossbar.v

@@ -0,0 +1,391 @@
+/*


I think this has to go in third_party, since it is not authored by a contributor.

This has been moved to third_party/verilog_axi.

proppy · 2025-04-09T15:11:38Z

xls/modules/zstd/data/comp_frame.x

@@ -0,0 +1,21 @@
+pub struct DataArray<BITS_PER_WORD: u32, LENGTH: u32>{


missing license?

Done - there's a license header now.

proppy · 2025-04-09T15:11:48Z

xls/modules/zstd/data/comp_frame_fse_repeated.x

@@ -0,0 +1,21 @@
+pub struct DataArray<BITS_PER_WORD: u32, LENGTH: u32>{


missing license?

Done - there's a license header now.

Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>

* Use materialize_internal_fifos when possible * Disable the above option for procs with loopback channels * Add missing module names * Add xls_fifo_wrapper verilog dependency to the synthesis of the procs without materialized internal fifos Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>

No longer applicable - there are no CC tests in ZSTD module Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>

Signed-off-by: Wojciech Sipak <wsipak@antmicro.com>

Signed-off-by: Robert Winkler <rwinkler@antmicro.com>

Co-authored-by: Szymon Gizler <sgizler@antmicro.com>

Signed-off-by: Wojciech Sipak <wsipak@antmicro.com>

proppy · 2025-05-20T07:47:50Z

xls/modules/zstd/memory/rtl/mem_writer_wrapper.v

@@ -0,0 +1,226 @@
+// Copyright 2024 The XLS Authors


make sure to wrap line to 100 characters in that file.

proppy · 2025-05-20T12:24:30Z

xls/modules/zstd/README.md

+        which did not have the `last` flag set
+*   DecoderMux
+    *   At the beginning of the simulation or after receiving
+        `ExtendedBlockDataPacket` with `last` and `last_block` (decoding new ZSTD


justify to 80 columns?

Signed-off-by: Wojciech Sipak <wsipak@antmicro.com>

proppy · 2025-05-20T13:00:27Z

dependency_support/pip_requirements.in

 z3-solver==4.14.0.0
+pytest==8.2.2
+cocotb==1.9.0
+cocotbext-axi==0.1.24


I don't think this is available in our internal repository, so it would be better to untie this dependency for now and remove the associated script from the PR and import them separately.

proppy · 2025-05-20T13:02:10Z

dependency_support/pip_requirements.in

 # use pypi version
 z3-solver==4.14.0.0
+pytest==8.2.2
+cocotb==1.9.0


FYI, internally we are on v1.7.1, so if there is anything that's specific to 1.9.0, will need to upgrade the internal copy of cocotb first; as I commented elsewhere it might be better to untie this and the associated python tests from this PR and import this separately.

proppy · 2025-05-20T13:02:43Z

dependency_support/pip_requirements.in

+pytest==8.2.2
+cocotb==1.9.0
+cocotbext-axi==0.1.24
+cocotb_bus==0.2.1


Same here, this is not available yet internally and will need to be imported separately.

proppy · 2025-05-20T13:03:17Z

dependency_support/pip_requirements.in

+cocotb==1.9.0
+cocotbext-axi==0.1.24
+cocotb_bus==0.2.1
+zstandard==0.23.0


FYI, internally this is on 0.19.0

proppy · 2025-05-20T13:03:43Z

dependency_support/pip_requirements.in

 # We build most of z3 ourselves but building python is really complicated. Just
 # use pypi version
 z3-solver==4.14.0.0
+pytest==8.2.2


Internally this is on 8.0.2

proppy · 2025-05-20T13:28:01Z

xls/modules/zstd/BUILD

@@ -17,11 +17,11 @@
 load("@rules_hdl//place_and_route:build_defs.bzl", "place_and_route")


consider adding https://google.github.io/xls/bazel_rules_macros/#xls_dslx_fmt_test to enforce DSLX formatting.

proppy · 2025-05-20T13:29:20Z

.github/workflows/modules-zstd.yml

I think you also want to consider running https://google.github.io/xls/bazel_rules_macros/#xls_dslx_fmt_test to enforce DSLX formatting as part of the CI.

proppy · 2025-05-21T13:27:46Z

super seeded by #2230

tmichalak force-pushed the zstd_compressed_block_dec branch from 47ab08c to 50f6140 Compare January 15, 2025 20:51

lpawelcz force-pushed the zstd_compressed_block_dec branch from 50f6140 to 36bc838 Compare January 16, 2025 07:19

This was referenced Jan 20, 2025

modules/zstd: Add sequence decoding #1362

Closed

modules/zstd: Add literals decoding #1430

Closed

proppy mentioned this pull request Jan 27, 2025

modules/zstd: Rework ZSTD Decoder #1654

Closed

7 tasks

rw1nkler force-pushed the zstd_compressed_block_dec branch from 36bc838 to 16cd455 Compare January 29, 2025 21:28

rw1nkler mentioned this pull request Jan 30, 2025

Huge RAM consumption in DSLX interpreter #1897

Open

lpawelcz mentioned this pull request Feb 5, 2025

Add cocotb tests for AXI peripherals #1616

Closed

proppy reviewed Mar 6, 2025

View reviewed changes

xls/dslx/stdlib/BUILD Show resolved Hide resolved

proppy reviewed Mar 6, 2025

View reviewed changes

rw1nkler force-pushed the zstd_compressed_block_dec branch from 16cd455 to cae18b3 Compare March 27, 2025 06:21

lpawelcz mentioned this pull request Apr 7, 2025

dslx/stdlib: Add missing acm_random DSLX library #2033

Merged

proppy added the Reviewing Internally label Apr 9, 2025

proppy reviewed Apr 9, 2025

View reviewed changes

lpawelcz force-pushed the zstd_compressed_block_dec branch from cae18b3 to 70cbb9d Compare April 9, 2025 14:44

proppy approved these changes Apr 9, 2025

View reviewed changes

proppy reviewed Apr 9, 2025

View reviewed changes

lpawelcz and others added 6 commits May 16, 2025 17:03

modules/zstd: Remove RESET CSR

9678e28

Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>

CI/modules-zstd: Remove step for running CC tests

a59c71c

No longer applicable - there are no CC tests in ZSTD module Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>

modules/zstd: add license

2d2ce54

Signed-off-by: Wojciech Sipak <wsipak@antmicro.com>

Remove cocotb test of the zstd decoder

2519c21

Signed-off-by: Robert Winkler <rwinkler@antmicro.com>

modules: zstd: formatting and wording changes

aadd59e

Co-authored-by: Szymon Gizler <sgizler@antmicro.com>

wsipak force-pushed the zstd_compressed_block_dec branch from 1f89c87 to 412eb98 Compare May 16, 2025 15:06

wsipak added 2 commits May 16, 2025 18:33

resolve linter errors

437dbc7

Signed-off-by: Wojciech Sipak <wsipak@antmicro.com>

use newline before and after headings and lists

8b23ebd

wsipak force-pushed the zstd_compressed_block_dec branch 3 times, most recently from de1ee9a to d77223d Compare May 19, 2025 16:04

tmichalak force-pushed the zstd_compressed_block_dec branch from d77223d to b327068 Compare May 20, 2025 07:14

proppy reviewed May 20, 2025

View reviewed changes

wsipak force-pushed the zstd_compressed_block_dec branch 2 times, most recently from cb9d65d to 35a215a Compare May 20, 2025 09:46

proppy reviewed May 20, 2025

View reviewed changes

resolve linter errors

6836389

Signed-off-by: Wojciech Sipak <wsipak@antmicro.com>

wsipak force-pushed the zstd_compressed_block_dec branch from 35a215a to 6836389 Compare May 20, 2025 12:38

proppy reviewed May 20, 2025

View reviewed changes

wsipak mentioned this pull request May 21, 2025

modules/zstd: Add support for decoding compressed blocks (DSLX part) #2230

Closed

proppy closed this May 21, 2025

rw1nkler mentioned this pull request May 30, 2025

modules/zstd: Add support for decoding compressed blocks (DSLX part) #2296

Merged

		@@ -0,0 +1,21 @@
		pub struct DataArray<BITS_PER_WORD: u32, LENGTH: u32>{

		@@ -17,11 +17,11 @@
		load("@rules_hdl//place_and_route:build_defs.bzl", "place_and_route")

Comments

Conversation

lpawelcz commented Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

google-cla bot commented Jan 15, 2025

Uh oh!

proppy commented Jan 27, 2025

Uh oh!

lpawelcz commented Feb 5, 2025

Uh oh!

Uh oh!

proppy Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

proppy commented Mar 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lpawelcz commented Apr 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lpawelcz commented Jan 15, 2025 •

edited

Loading

proppy Mar 6, 2025 •

edited

Loading